-
Notifications
You must be signed in to change notification settings - Fork 898
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add registered_provider_plugins to Vmdb::Plugins #13983
Add registered_provider_plugins to Vmdb::Plugins #13983
Conversation
this allows for querying which providers are registered via plugins it also provides a helper method to query for provider names of all registered providers
lib/vmdb/plugins.rb
Outdated
def register_automate_domains(engine) | ||
Dir.glob(engine.root.join("content", "automate", "*")).each do |domain_directory| | ||
@registered_automate_domains << AutomateDomain.new(domain_directory) | ||
def provider_plugin_names |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We usually implement something like this as:
def provider_plugin_names(refresh = false)
@provider_plugin_names = nil if refresh
@provider_plugin_names ||= .....
end
lib/vmdb/plugins.rb
Outdated
def register_provider_plugin(engine) | ||
if engine.class.name.start_with?("ManageIQ::Providers::") | ||
@registered_provider_plugins << engine | ||
@provider_plugins_names = nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels strange to wipe this cache in this method. Based on the comment above, you could instead call provider_plugin_names(true)
lib/vmdb/plugins.rb
Outdated
@registered_automate_domains << AutomateDomain.new(domain_directory) | ||
def provider_plugin_names | ||
@provider_plugins_names ||= registered_provider_plugins.collect do |plugin| | ||
ManageIQ::Providers::Inflector.provider_name(plugin).downcase.to_sym |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be underscore
rather than downcase
to deal with things like AnsibleTower
in the future?
@bdunne addressed your feedback - thanks for catching the |
Checked commits durandom/manageiq@edc86e2~...9e6d72f with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 LGTM. @Fryguy Any concerns?
this allows for querying which providers are registered via plugins.
it also provides a helper method to query for provider names of all registered providers
extracted from #13942
@miq-bot add_labels pluggable providers, enhancement
@miq-bot assign @bdunne